Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sorting issue with memo and virtualization #310

Merged
merged 1 commit into from
Jan 10, 2023

Conversation

duckboy81
Copy link
Contributor

Problem:
When using memoMode = "row" and table virtualization, when sorting a column, row positioning may not be correctly updated if the row existed (in DOM?) before AND after the sort.

Fix:
Add additional check to propsAreEqual function for Memo_MRT_TableBodyRow to check if the row index has changed.

Problem:
When using `memoMode = "row"` and table virtualization, when sorting a column, row positioning may not be correctly updated if the row existed (in DOM?) before the sort AND after the sort.

Fix:
Add additional check to `propsAreEqual` function for the `Memo_MRT_TableBodyRow` to check if the row index has changed.
@vercel
Copy link

vercel bot commented Jan 10, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
material-react-table ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Jan 10, 2023 at 0:45AM (UTC)
material-react-table-storybook ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Jan 10, 2023 at 0:45AM (UTC)

@duckboy81
Copy link
Contributor Author

duckboy81 commented Jan 10, 2023

See issue here: https://codesandbox.io/s/lucid-bessie-vohfsb?file=/src/TS.tsx

EDIT: Sort the first name column a few times and you'll see some discrepancies.

@KevinVandy
Copy link
Owner

Wonder if we should also add the row id to the list to check

@KevinVandy KevinVandy merged commit aa72893 into KevinVandy:main Jan 10, 2023
@duckboy81
Copy link
Contributor Author

Wonder if we should also add the row id to the list to check

I can't say I'm too acquainted with all the inner workings of the project, but it would seem row.id probably wouldn't change unless the entire row object is rebuilt with a change to the data prop.

I think the existing object reference check prev.row === next.row is probably good enough.

Thanks for this project btw, amazing OOTB capabilities.

@duckboy81 duckboy81 deleted the patch-4 branch January 11, 2023 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants